Skip to content

fix: SCIM reconciliation fixes and diagnostic logging#65

Merged
TerrifiedBug merged 1 commit intomainfrom
fix/scim-reconciliation-fixes
Mar 8, 2026
Merged

fix: SCIM reconciliation fixes and diagnostic logging#65
TerrifiedBug merged 1 commit intomainfrom
fix/scim-reconciliation-fixes

Conversation

@TerrifiedBug
Copy link
Copy Markdown
Owner

Summary

  • Adds diagnostic logging to OIDC group reconciliation chain to troubleshoot remote deployment where SSO users aren't being mapped to teams
  • Logs at each decision point: token groups extraction, SCIM+OIDC union, loaded mappings, desired state, existing members

Context

After merging PR #64 (SCIM reconciliation redesign), SSO login with OIDC+SCIM enabled is still not mapping users to their groups. Since the deployment is remote, these logs will reveal exactly where the chain breaks.

What to look for in logs

  1. [oidc] User <email> groups (claim "groups"): [...] — groups from OIDC token
  2. [oidc] User <email> scimEnabled=true, final groups: [...] — after SCIM union
  3. [reconcile] userId=..., userGroups=[...], mappings=[...] — loaded mappings config
  4. [reconcile] desiredTeamRoles=[...] — computed desired state
  5. [reconcile] existing group_mapping members=[...] — current DB state

Test plan

  • Deploy to remote server
  • Log in with SSO user
  • Check server logs for [oidc] and [reconcile] entries
  • Identify which step fails and fix accordingly

Temporary debug logs at each decision point in the reconciliation chain:
- auth.ts: log final group list and scimEnabled flag
- group-mappings.ts: log loaded mappings, desired state, existing members
@github-actions github-actions bot added the fix label Mar 8, 2026
@TerrifiedBug TerrifiedBug merged commit 6d4ac57 into main Mar 8, 2026
6 checks passed
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 8, 2026

Greptile Summary

This PR adds four console.log diagnostic statements to the OIDC group-sync reconciliation chain — one in src/auth.ts after the SCIM+OIDC union is computed, and three in src/server/services/group-mappings.ts covering the loaded mappings, the desired-state map, and the existing DB records. No logic is changed; this is purely observability scaffolding to diagnose why SSO users aren't being mapped to teams in the remote deployment.

Key observations:

  • Placement of all four logs is correct relative to the reconciliation steps they instrument.
  • user.email is already logged at the earlier token-groups step (pre-existing log on line 256 of auth.ts); the new log follows the same pattern and adds scimEnabled state and the post-union group list.
  • The allMappings log in reconcileUserTeamMemberships dumps the complete system-wide RBAC mapping table on every call — this function is invoked for both OIDC logins and SCIM membership changes, so the full config will appear in logs on every event. Scoping the log to only the matched mappings would reduce noise and limit config exposure in logs (see inline comment).

Confidence Score: 4/5

  • Safe to merge — changes are purely diagnostic logging with no logic alterations; one minor issue with log scope.
  • All four log statements are correctly placed and do not alter any control flow or data. No tokens, passwords, or encrypted secrets are logged. The only concern is that the allMappings dump in group-mappings.ts emits the full system-wide RBAC config table on every reconcile call (including SCIM events), which is noisier and broader in scope than necessary. This is a minor/style issue that doesn't affect correctness or security.
  • src/server/services/group-mappings.ts — review the allMappings log scope at line 59

Important Files Changed

Filename Overview
src/auth.ts Adds one diagnostic console.log after the SCIM+OIDC union step, logging user email and final group list. Placement is correct — after the union is computed but before reconciliation is called. Note: user.email was already logged at the earlier token-groups step (pre-existing), so this follows an established pattern.
src/server/services/group-mappings.ts Adds three diagnostic console.log calls: full allMappings dump after load, desiredTeamRoles after compute, and existing members after DB fetch. The allMappings log dumps the complete system-wide RBAC mapping table (all teams/groups/roles) on every invocation, including SCIM-triggered reconciliations — potentially noisy and exposes full org RBAC config to server logs.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[OIDC Callback] --> B[Extract groups from token claim]
    B --> C{SCIM enabled?}
    C -->|Yes| D[Load SCIM group memberships from DB]
    D --> E[Union of SCIM groups and token groups]
    C -->|No| F[Use token groups directly]
    E --> G[LOG: final groups list]
    F --> G
    G --> H[reconcileUserTeamMemberships]
    H --> I[Load all group mappings from SystemSettings]
    I --> J[LOG: userId + userGroups + full mappings dump]
    J --> K[Compute desired team roles map]
    K --> L[LOG: desired roles]
    L --> M[Fetch existing group-mapped memberships from DB]
    M --> N[LOG: existing members]
    N --> O[Diff desired vs existing]
    O --> P[Create new memberships]
    O --> Q[Update changed roles]
    O --> R[Delete stale memberships]
Loading

Last reviewed commit: 3fa3341

userGroupNames: string[],
): Promise<void> {
const allMappings = await loadGroupMappings();
console.log(`[reconcile] userId=${userId}, userGroups=${JSON.stringify(userGroupNames)}, mappings=${JSON.stringify(allMappings)}`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Full system-wide RBAC config dumped on every reconcile call

allMappings contains every group-to-team mapping in the system — not just the ones relevant to this user. This function is called on every SSO login and on every SCIM group membership change, so in a busy deployment you'll get a full dump of your entire RBAC configuration in the logs on each event.

Two considerations:

  1. Verbosity: in a deployment with many SCIM users this will generate very noisy logs and make the entries you actually care about harder to find.
  2. Scope of exposure: server logs that contain the full mapping table (all teamIds, group names, and assigned roles) represent a broader blast radius if log storage is ever misconfigured or accessed by the wrong party, compared to logging only the subset of mappings that matched the current user's groups.

Consider scoping the log to the mappings that actually matched this user:

const matchedMappings = allMappings.filter(m => userGroupNames.includes(m.group));
console.log(`[reconcile] userId=${userId}, userGroups=${JSON.stringify(userGroupNames)}, matchedMappings=${JSON.stringify(matchedMappings)}`);

This still reveals which mappings fired and which groups matched, giving you all the diagnostic signal you need without dumping the full config table every time.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/server/services/group-mappings.ts
Line: 59

Comment:
**Full system-wide RBAC config dumped on every reconcile call**

`allMappings` contains every group-to-team mapping in the system — not just the ones relevant to this user. This function is called on every SSO login *and* on every SCIM group membership change, so in a busy deployment you'll get a full dump of your entire RBAC configuration in the logs on each event.

Two considerations:
1. **Verbosity**: in a deployment with many SCIM users this will generate very noisy logs and make the entries you actually care about harder to find.
2. **Scope of exposure**: server logs that contain the full mapping table (all teamIds, group names, and assigned roles) represent a broader blast radius if log storage is ever misconfigured or accessed by the wrong party, compared to logging only the subset of mappings that matched the current user's groups.

Consider scoping the log to the mappings that actually matched this user:
```typescript
const matchedMappings = allMappings.filter(m => userGroupNames.includes(m.group));
console.log(`[reconcile] userId=${userId}, userGroups=${JSON.stringify(userGroupNames)}, matchedMappings=${JSON.stringify(matchedMappings)}`);
```
This still reveals which mappings fired and which groups matched, giving you all the diagnostic signal you need without dumping the full config table every time.

How can I resolve this? If you propose a fix, please make it concise.

@TerrifiedBug TerrifiedBug deleted the fix/scim-reconciliation-fixes branch March 8, 2026 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant